Repository navigation
Allow public tuning of the cub::DeviceRunLengthEncode::NonTrivialRuns - #9347
Conversation
SummaryThis PR productizes the tuning API for DeviceRunLengthEncode::NonTrivialRuns by introducing a public cub::RleNonTrivialRunsPolicy and making related dispatch/tuning machinery use that public type (fixes
ChangesPublic API Additions
Deprecations
Implementation & Dispatch Updates
Documentation
Tests
Other
important: WalkthroughThis PR makes the NonTrivialRuns tuning API public by adding cub::RleNonTrivialRunsPolicy, refactors AgentRlePolicy into detail::agent_rle_policy with a deprecated alias, updates all selector and dispatch code to use the new public type, and adds compile-time and runtime tests and tuning examples. ChangesRLE NonTrivialRuns Tuning API Productization
Assessment against linked issues
Suggested reviewers
Comment |
This comment has been minimized.
This comment has been minimized.
2c0c1a6 to
21026ad
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
cub/cub/device/dispatch/tuning/tuning_rle_non_trivial_runs.cuh (4)
34-34: 💤 Low valuesuggestion: Add explicit
@brieftag for consistency.The Doxygen comment lacks an explicit
@brieftag. While guidelines specifically require this for functions, adding it for the struct improves consistency.-//! The tuning policy for `@ref` DeviceRunLengthEncode::NonTrivialRuns. +//! `@brief` The tuning policy for `@ref` DeviceRunLengthEncode::NonTrivialRuns.Source: Coding guidelines
392-406: 💤 Low valuesuggestion: Consider marking
make_default_policyasnoexcept.The function performs only compile-time arithmetic and aggregate initialization. If
default_reduce_by_key_delay_constructor_policyis alsonoexcept(or constexpr-evaluated), this function doesn't throw and should be markednoexceptper guidelines.Source: Coding guidelines
408-606: 💤 Low valuesuggestion: Consider marking
operator()asnoexcept.The function branches on compute capability and returns aggregate-initialized policy objects. If all paths are no-throw (aggregate initialization of POD-like fields), mark it
noexceptper guidelines.Source: Coding guidelines
615-627: 💤 Low valuesuggestion: Consider marking
operator()asnoexcept.Delegates to
selector(cc)which performs compile-time selection. If the selector and construction are no-throw, marknoexceptper guidelines.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 76847392-186d-42b7-9e0a-b9e9eaca23aa
📒 Files selected for processing (9)
cub/benchmarks/bench/run_length_encode/non_trivial_runs.cucub/cub/agent/agent_rle.cuhcub/cub/device/device_run_length_encode.cuhcub/cub/device/dispatch/dispatch_rle.cuhcub/cub/device/dispatch/tuning/tuning_rle_non_trivial_runs.cuhcub/test/catch2_test_device_rle_custom_policy_hub.cucub/test/catch2_test_device_run_length_encode_env.cucub/test/catch2_test_device_run_length_encode_env_api.cucub/test/catch2_test_device_run_length_encode_non_trivial_runs.cu
✅ Files skipped from review due to trivial changes (2)
- cub/cub/device/device_run_length_encode.cuh
- cub/test/catch2_test_device_rle_custom_policy_hub.cu
🚧 Files skipped from review as they are similar to previous changes (6)
- cub/benchmarks/bench/run_length_encode/non_trivial_runs.cu
- cub/test/catch2_test_device_run_length_encode_non_trivial_runs.cu
- cub/test/catch2_test_device_run_length_encode_env.cu
- cub/cub/agent/agent_rle.cuh
- cub/test/catch2_test_device_run_length_encode_env_api.cu
- cub/cub/device/dispatch/dispatch_rle.cuh
| //! The tuning policy for @ref DeviceRunLengthEncode::NonTrivialRuns. | ||
| struct RleNonTrivialRunsPolicy | ||
| { | ||
| int threads_per_block; //!< Number of threads in a CUDA block | ||
| int items_per_thread; //!< Number of items processed per thread | ||
| BlockLoadAlgorithm load_algorithm; //!< The @ref BlockLoadAlgorithm used for loading items from global memory | ||
| CacheLoadModifier load_modifier; //!< The @ref CacheLoadModifier used for loading items from global memory | ||
| bool store_with_time_slicing; //!< Whether to time-slice shared memory for store transpositions | ||
| BlockScanAlgorithm scan_algorithm; //!< The @ref BlockScanAlgorithm used for scanning | ||
|
|
||
| //! The @ref LookbackDelayPolicy controlling the delay between lookback iterations | ||
| LookbackDelayPolicy lookback_delay = {LookbackDelayAlgorithm::fixed_delay, 350, 450}; | ||
|
|
||
| [[nodiscard]] _CCCL_HOST_DEVICE_API constexpr friend bool | ||
| operator==(const RleNonTrivialRunsPolicy& lhs, const RleNonTrivialRunsPolicy& rhs) noexcept | ||
| { | ||
| return lhs.threads_per_block == rhs.threads_per_block && lhs.items_per_thread == rhs.items_per_thread | ||
| && lhs.load_algorithm == rhs.load_algorithm && lhs.load_modifier == rhs.load_modifier | ||
| && lhs.store_with_time_slicing == rhs.store_with_time_slicing && lhs.scan_algorithm == rhs.scan_algorithm | ||
| && lhs.lookback_delay == rhs.lookback_delay; | ||
| } | ||
|
|
||
| [[nodiscard]] _CCCL_HOST_DEVICE_API constexpr friend bool | ||
| operator!=(const RleNonTrivialRunsPolicy& lhs, const RleNonTrivialRunsPolicy& rhs) noexcept | ||
| { | ||
| return !(lhs == rhs); | ||
| } | ||
|
|
||
| #if _CCCL_HOSTED() | ||
| friend ::std::ostream& operator<<(::std::ostream& os, const RleNonTrivialRunsPolicy& p) | ||
| { | ||
| return os | ||
| << "RleNonTrivialRunsPolicy { .threads_per_block = " << p.threads_per_block | ||
| << ", .items_per_thread = " << p.items_per_thread << ", .load_algorithm = " << p.load_algorithm | ||
| << ", .load_modifier = " << p.load_modifier << ", .store_with_time_slicing = " << p.store_with_time_slicing | ||
| << ", .scan_algorithm = " << p.scan_algorithm << ", .lookback_delay = " << p.lookback_delay << " }"; | ||
| } | ||
| #endif // _CCCL_HOSTED() | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other public policy struct naming patterns in CUB device headers
rg -n "^struct [A-Z][a-zA-Z]*Policy" cub/cub/device/ --type=cpp -C2Repository: NVIDIA/cccl
Length of output: 37
Fix CCCL naming/Doxygen compliance in cub/cub/device/dispatch/tuning/tuning_rle_non_trivial_runs.cuh
RleNonTrivialRunsPolicymember names (threads_per_block,items_per_thread,store_with_time_slicing,lookback_delay) don’t use the required trailing_postfix for data members.- The Doxygen comment for
RleNonTrivialRunsPolicylacks//!@brief`` (it only says “The tuning policy…”). - Add
noexcepttopolicy_selector::make_default_policy(...)andpolicy_selector::operator()( ::cuda::compute_capability ) constwhen they don’t throw.
Source: Coding guidelines
🥳 CI Workflow Results🟩 Finished in 2h 43m: Pass: 100%/287 | Total: 12d 11h | Max: 2h 42m | Hits: 15%/1399288See results here. |
Fixes: #8577
New public entities
cub::RleNonTrivialRunsPolicyint threads_per_blockint items_per_threadBlockLoadAlgorithm load_algorithmCacheLoadModifier load_modifierbool store_with_time_slicingBlockScanAlgorithm scan_algorithmLookbackDelayPolicy lookback_delay